Skip to content

Conversation

@MarcelloPerathoner
Copy link
Contributor

@MarcelloPerathoner MarcelloPerathoner commented Dec 22, 2025

Issue

Fixes #7307 - CI runs up from 0:45h to 4:30h

Improvements

Parallel execution of cucumber tests tremendously improves running times.

Note: when using --parallel N make sure there are N contiguous free ports at the
configured port number (eg. at ports 5000--5000+N).

A lot more can be configured with profiles (in cucumber.mjs). Usage examples:

$ npx cucumber-js -p home -p ch -p mmap --format html:/tmp/testlog.html --parallel 8
$ npx cucumber-js -p github -p mld -p directly --parallel 8
$ npm test -- --parallel $JOBS --fail-fast

Github step summaries display the test results:

Screenshot 2026-01-08 at 21-33-21 WIP Parallel cucumbers · Project-OSRM_osrm-backend@6b55220

On github CI full logs, also containing the outputs from all binaries, are published to
https://reports.cucumber.io/.

Code refactored and simplified. ~150 LoC less.

Other Fixes

During development other problems were found and fixed:

  • The datastore load method now works as expected: one and the same osrm-routed will
    be running in the background for the whole duration of the test. Previous behaviour: for
    each scenario an osrm-routed would be spawned, loaded with datastore, and then killed
    again.

  • The data watchdog in osrm-routed now will emit the message: 'updated facade' right
    after the update (previous behaviour: right before the update), so that it can be used
    for process synchronization.

Breaking changes

If you run the tests using the npx cucumber-js ... calling method you must add either
-p home or -p github as the first profile, eg.:

$ npx cucumber-js -p home -p mld -p mmap --parallel 8

instead of:

$ OSRM_LOAD_METHOD=mmap npx cucumber-js -p mld

Old cached files are not cleaned automatically anymore because that interfered with
concurrency. There is no hook in cucumber that runs before the workers start.

We don't support changing the load method in mid-run anymore. (eg. "Given data is loaded
with datastore" no longer works). Reason: With the datastore load method an
osrm-routed will be constantly running in the background. No second osrm-routed can
open the same port again.

Retired Environment Variables

  • OSRM_CONNECTION_RETRIES
  • OSRM_CONNECTION_EXP_BACKOFF_COEF

Other changes

Removed one cache level. CH and MLD are processed and cached separately.

The outputs of osrm binaries are not written into lots of small files anymore. Instead
use the --format html:test/logs/testlog.html commandline argument to get one big HTML
file with all outputs embedded.

New configuration variables

New environment variables and/or cucumber worldParameters entries were introduced. The
whole configuration is done in cucumber.mjs.

Environment Variable worldParameters Defaults to
CUCUMBER_TIMEOUT timeout 5000 Scenario timeout in ms.
CUCUMBER_HTTP_TIMEOUT httpTimeout 2000 HTTP timeout in ms.
CUCUMBER_TEST_PATH testPath test The test directory
CUCUMBER_PROFILES_PATH profilesPath profiles The profiles directory
CUCUMBER_LOGS_PATH logsPath test/logs The logs directory
CUCUMBER_CACHE_PATH cachePath test/cache The cache directory
OSRM_BUILD_DIR buildPath build Path to the binaries
OSRM_LOAD_METHOD loadMethod datastore Data load method
OSRM_ALGORITHM algorithm ch Routing algorithm
OSRM_IP ip 127.0.0.1 IP Address
OSRM_PORT port 5000 IP Port

New tags

Tag A scenario thus tagged ...
@isolated will not run while any other scenario is running in parallel
@with_(datastore|directly|mmap) will be executed iff the load method matches
@no_(datastore|directly|mmap) will be executed unless the load method matches
@with_(ci|mld) will be executed iff the algorithm matches
@no_(ci|mld) will be executed unless the algorithm matches

Tasklist

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

@MarcelloPerathoner MarcelloPerathoner changed the title WIP: Speed up cucumber tests WIP: Parallel cucumbers Dec 24, 2025
@afarber
Copy link
Contributor

afarber commented Dec 27, 2025

Thanks for working on speeding up the CI! The parallel cucumber approach looks promising.

One concern: could we keep the CI matrix changes out of this PR? Switching from macos-13 to macos-15-intel drops test coverage for users on Ventura/Sonoma, and adding clang-20 (which isn't released yet) seems unrelated to the parallelization work. These changes could be discussed separately if there's a reason to drop older platform support.

Also spotted a few bugs in osrm_loader.js - there are references to an undefined err variable in the error handlers (lines 45, 118, 132) that would crash if osrm-routed fails.

@MarcelloPerathoner
Copy link
Contributor Author

The PR is still WIP. I will cleanup before releasing it.

BTW clang-21 is current. The reason to drop macos-13 is that github does not support it anymore.

);
callback();
});
// process.on('SIGIO', this.resolve);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left-over?

@TheMarex
Copy link
Member

TheMarex commented Jan 8, 2026

Do you think the non-cucumber changes (datastore, routed, etc) can be broken out independently without messing with the tests? It is always quite the challenge to review large changes like this so I would appreciate the extra effort if possible. 🙏

@MarcelloPerathoner
Copy link
Contributor Author

The shared memory changes could easily go elsewhere, as they are not the primary reason for sync loss between the test suite and routed. But the change in routed is critical (and trivial too).

The changes to the github workflow should go elsewhere. I'm just playing around to get windows to run the test suite too. See if any surprises come up.

@MarcelloPerathoner MarcelloPerathoner force-pushed the Fix-cucumber-routed-startup-timeout branch 2 times, most recently from e09c32e to 6b55220 Compare January 8, 2026 20:23
@MarcelloPerathoner MarcelloPerathoner changed the title WIP: Parallel cucumbers Parallel cucumbers Jan 8, 2026
@MarcelloPerathoner
Copy link
Contributor Author

Ready for review. I have removed everything that is not strictly necessary. The shared memory fixes for macOS and Windows will go into another PR. Until that macOS tests with datastore will be flaky.

@MarcelloPerathoner MarcelloPerathoner force-pushed the Fix-cucumber-routed-startup-timeout branch from 0f33530 to 1e1d5fa Compare January 14, 2026 07:33
@afarber
Copy link
Contributor

afarber commented Jan 14, 2026

Looks like your PR #7321 resolved the root cause - nice work.

I notice you've added back setDefaultTimeout() with CUCUMBER_TIMEOUT support, but since all the CUCUMBER_TIMEOUT: 60000 entries were removed from the workflow, the env var is never set. It will always default to 5000ms, which is also Cucumber's built-in default.

You could simplify by removing setDefaultTimeout() again.

@MarcelloPerathoner
Copy link
Contributor Author

Thanks. I put the timeout back in case somebody might want it badly, though I strongly advise against using it. Currently everything works fine with the default timeout.

Copy link
Member

@TheMarex TheMarex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. 🙇 Tried it locally and while I'll miss the never ending row of green dots broken by the inevitable red F this is so nice. Massive amount of work, appreciate it. 🙌 Only minor questions / comments.

@@ -1,33 +1,63 @@
@datastore @options @help
@datastore @options @help @isolated @no_datastore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm I'm not so I follow: What does @datastore in conjunction with @no_datastore do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@datastore is a legacy tag, means: run all test concerning osrm-datastore. Those tests do ugly things like: osrm-datastore --spring-clean. So they cannot run while other tests are actually using the datastore. @no_datastore is a new tag, meaning: don't run if the selected load method is datastore. A bit confusing ...

});

Given(/^the node map$/, function (docstring, callback) {
const q = d3.queue();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm interesting - any idea why this used to be async?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it was easier to call it this way instead of thru a chain of callbacks?

let query = '';
if (params.coordinates !== undefined) {
// FIXME this disables passing the output if its a default
// Remove after #2173 is fixed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So actually that referenced issue is fixed. Though honestly it may be fine to leave it implicitly as is.

@@ -1,4 +1,4 @@
@routing @testbot @alternative
@routing @testbot @alternative @isolated @todo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the todo here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That test must be rewritten. The result is unstable.

@routing @load @testbot @todo @isolated @no_datastore
Feature: Ways of loading data

# TEST BROKEN: we don't support changing the load method in mid-run.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't start a new osrm-routed does this work though? The core here is I think the spring clean.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for that test has been removed. While it may be possible to switch between mmap and directly I don't see the point. Most of those tests are duplicates.


set -o errexit
set -o pipefail
set -o nounset
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left-over from debugging?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$GITHUB_STEP_SUMMARY may be undefined.

@MarcelloPerathoner MarcelloPerathoner force-pushed the Fix-cucumber-routed-startup-timeout branch from eb6f405 to c765fee Compare January 16, 2026 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI runs up from 0:45h to 4:30h

3 participants